Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce ImmutableEnumSetRules Refaster rule collection #1302

Merged
merged 3 commits into from
Oct 26, 2024

Conversation

mohamedsamehsalah
Copy link
Contributor

Suggested commit message

Introduce `ImmutableEnumSetRules` refaster rules

@mohamedsamehsalah mohamedsamehsalah added this to the 0.19.0 milestone Aug 20, 2024
Copy link

Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Copy link
Member

@rickie rickie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly some tweaks to Refaster rule names. Nice new collection @mohamedsamehsalah 🚀!

Added a commit.

Suggested commit message:

Introduce `ImmutableEnumSetRules` Refaster rule collection (#1302)

* Prefer {@link Sets#immutableEnumSet(Iterable)} for enum collections to take advantage of
* internally used {@link EnumSet}.
*/
static final class ImmutableEnumSetIterable<T extends Enum<T>> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The yet-to-be formalized Refaster naming algorithm would add the Sets here in front :).

*/
static final class ImmutableEnumSetIterable<T extends Enum<T>> {
@BeforeTemplate
ImmutableSet<T> before(Collection<T> elements) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the name wanted to change this to an Iterable, but that doesn't work here, the copyOf expects a Collection.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh, there's an Iterable-accepting overload. Just means we need to cover both cases.

}

ImmutableSet<ImmutableSet<RoundingMode>> testImmutableEnumSetIterable() {
return ImmutableSet.of(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can simplify the first two tests to simply have one test case :).

RoundingMode.UNNECESSARY)));
}

ImmutableSet<RoundingMode> testImmutableEnumSetSixElements() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For other similar rules we stopped at 5, but because its already here, might as well leave it in 😉.

@rickie rickie force-pushed the mohamedsamehsalah/immutable-enum-set branch from 8d0eeb7 to e80ef1d Compare October 23, 2024 08:52
Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

1 similar comment
Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202 Stephan202 force-pushed the mohamedsamehsalah/immutable-enum-set branch from e80ef1d to 81bf53e Compare October 26, 2024 11:09
Copy link
Member

@Stephan202 Stephan202 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased and added a commit.

*/
static final class ImmutableEnumSetIterable<T extends Enum<T>> {
@BeforeTemplate
ImmutableSet<T> before(Collection<T> elements) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh, there's an Iterable-accepting overload. Just means we need to cover both cases.

}

ImmutableSet<RoundingMode> testSetsImmutableEnumSetIterable() {
return Sets.immutableEnumSet(EnumSet.range(RoundingMode.UP, RoundingMode.UNNECESSARY));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use the shorter EnumSet.allOf(RoundingMode.class) here.

* Prefer {@link Sets#immutableEnumSet(Iterable)} for enum collections to take advantage of the
* internally used {@link EnumSet}.
*/
static final class SetsImmutableEnumSetIterableArray<T extends Enum<T>> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this one we'll likely name SetsImmutableEnumSetArraysAsList eventually; let's do already.

* Prefer {@link Sets#immutableEnumSet(Enum, Enum[])} for enum collections to take advantage of
* the internally used {@link EnumSet}.
*/
static final class SetsImmutableEnumSet6<T extends Enum<T>> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

>6 variants aren't rewritten; will add comment.

* Prefer {@link Sets#immutableEnumSet(Enum, Enum[])} for enum collections to take advantage of
* the internally used {@link EnumSet}.
*/
static final class ImmutableEnumSetVarArgs<T extends Enum<T>> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
static final class ImmutableEnumSetVarArgs<T extends Enum<T>> {
static final class SetsImmutableEnumSetVarArgs<T extends Enum<T>> {

* com.google.common.collect.ImmutableEnumSet}s.
*/
@OnlineDocumentation
final class ImmutableEnumSetRules {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we have a new rule collection, we should move the StreamToImmutableEnumSet rule from AssortedRules.

That rule also contains a warning that applies to the new rules.

Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202 Stephan202 changed the title Introduce ImmutableEnumSetRules refaster rules Introduce ImmutableEnumSetRules Refaster rule collection Oct 26, 2024
Copy link
Member

@rickie rickie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't have time to check the changes too thoroughly but they look good!

@Stephan202 Stephan202 force-pushed the mohamedsamehsalah/immutable-enum-set branch from 81bf53e to fd415f5 Compare October 26, 2024 11:36
Copy link

Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202 Stephan202 merged commit be6b17b into master Oct 26, 2024
16 checks passed
@Stephan202 Stephan202 deleted the mohamedsamehsalah/immutable-enum-set branch October 26, 2024 12:01
@mohamedsamehsalah
Copy link
Contributor Author

Thanks @Stephan202 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants